-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(sha512): add support for sha512 digest #1988
Conversation
Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
cc: @ktarplee |
Codecov Report
@@ Coverage Diff @@
## main #1988 +/- ##
==========================================
- Coverage 89.87% 89.86% -0.01%
==========================================
Files 158 158
Lines 26978 26984 +6
==========================================
+ Hits 24246 24249 +3
- Misses 2012 2013 +1
- Partials 720 722 +2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
digester := sha256.New() | ||
|
||
var digester hash.Hash | ||
if dstDigest.Algorithm() == godigest.SHA256 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to add support for SHA384 too:
else if dstDigest.Algorithm() == godigest.SHA384 {
digester = sha512.New384()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not need to reference sha256, sha384, or sha512 in your code. The godigest library does that for you.
digester := dstDigest.Algorithm().Hash() # hash.Hash is returned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course you should check that the algorithm is available with algDigest.Algorithm().Available()
(otherwise it will panic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main branch of godigest even allows one to register new algorithms if they choose. See this. So having the list of algorithms in Zot as a switch statement is not ideal. Let the library from open containers handle it for you.
Let's close this in favor of #2075 |
What type of PR is this?
Which issue does this PR fix:
What does this PR do / Why do we need it:
If an issue # is not available please add repro steps and logs showing the issue:
Testing done on this change:
Automation added to e2e:
Will this break upgrades or downgrades?
Does this PR introduce any user-facing change?:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.